[BD-24][TNL-7919] LTI Advantage Names and Roles provisioning service implementation direction - #142
Conversation
openedx-webhooks
added
blended
|
Thanks for the pull request, @giovannicimolin! I've created BLENDED-769 to keep track of it in Jira. More details are on the BD-24 project page. When this pull request is ready, tag your edX technical lead. |
| Placeholder. | ||
|
|
||
| Consequences | ||
| ============ |
There was a problem hiding this comment.
Another option is to pre-calculate the enrollment data (say on a nightly basis) and make a giant file that we push up to S3 and redirect to with an authorized link. That would allow us to serve the data quickly, though it would mean that the data was stale. Also, there's a good chance that getting a single file with a million enrollments would choke any potential consumer... but it is an option.
There was a problem hiding this comment.
We can actually pre-calculate multiple files in a scheduled task. And use those files as paginations. That will not decrease the number of requests, but requests will not do any db queries.
| ~~~~~~~~~~~~~~~~~~~~ | ||
| Placeholder. | ||
|
|
||
| Implement NRPS gated by course |
There was a problem hiding this comment.
This will be applicable for any implementation if we create xblock level flag if NRPS is enabled or not.
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| Placeholder. | ||
|
|
||
| Implement NRPS limiting the context of the data retrieved |
There was a problem hiding this comment.
I am not sure what context limiting means and how that helps. Is it reducing the number of fields per record?
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||
| Placeholder. | ||
|
|
||
| Implement NRPS for courses up to a predefined number of students |
There was a problem hiding this comment.
This seems a good option to me. One suggestion I have is to use the NRPS spec (pagination) based API for courses up to a fixed number of students. And Implement the pre-calculation approach suggested by ormsbee for other courses.
| Placeholder. | ||
|
|
||
| Consequences | ||
| ============ |
There was a problem hiding this comment.
We can actually pre-calculate multiple files in a scheduled task. And use those files as paginations. That will not decrease the number of requests, but requests will not do any db queries.
|
So before we dive too deeply on the merits of particular approaches, I'd really like this ADR to articulate the use cases we're solving for in implementing this part of the spec for Open edX courses. There are certain use cases I could imagine (e.g. check to see if this user is really allowed to use some service), for which having one-day-stale data may be unacceptable. If it's mostly used by proctoring and other premium services, maybe limiting it to a subset of enrollments makes sense. Those are completely unvalidated examples I made up on the spot, but my point is that I really don't know how different categories of clients are going to use this information, so it's hard to frame any discussion around technical tradeoffs. |
| Consequences | ||
| ============ | ||
|
|
||
| Depends on choosen approach. No newline at end of file |
There was a problem hiding this comment.
| Depends on choosen approach. | |
| Depends on chosen approach. |
Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
nedbat
left a comment
nedbat
left a comment
There was a problem hiding this comment.
Just some tweaks of English to clarify who is acting.
Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
Co-authored-by: Ned Batchelder <ned@nedbatchelder.com>
|
@nedbat I addressed your comments in the last commit. |
6 participants
This PR adds an ADR to discuss the possible implementations of the LTI NRPS service.
Related PRs:
#124
https://github.com/edx/edx-platform/pull/25843
https://github.com/edx/edx-platform/pull/25844
@ormsbee Since you raised the scalability issue of the API.
@nedbat @shimulch